Skip to content

Update ACL during job reset#38741

Merged
pankajkoti merged 7 commits into
apache:mainfrom
SubhamSinghal:bugfix/create-job-does-not-update-acl
Apr 10, 2024
Merged

Update ACL during job reset#38741
pankajkoti merged 7 commits into
apache:mainfrom
SubhamSinghal:bugfix/create-job-does-not-update-acl

Conversation

@SubhamSinghal

@SubhamSinghal SubhamSinghal commented Apr 4, 2024 •
edited by eladkal
Loading

Copy link
Copy Markdown
Contributor

closes: #38721

@SubhamSinghal

SubhamSinghal commented Apr 4, 2024

Copy link
Copy Markdown
Contributor Author

@Lee-W Please review changes.

@Lee-W Lee-W self-requested a review April 5, 2024 15:59
@subham611

subham611 commented Apr 7, 2024

Copy link
Copy Markdown
Contributor

@Lee-W Wait for CI images step is failing. How should I fix this, or simply rerun would work?

@Lee-W

Lee-W commented Apr 7, 2024

Copy link
Copy Markdown
Member

@Lee-W Wait for CI images step is failing. How should I fix this, or simply rerun would work?

Let me trigger rerun and see how it works

@subham611

subham611 commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

@Lee-W How to fix Wait for CI images error. Need help here.

@potiuk potiuk force-pushed the bugfix/create-job-does-not-update-acl branch from 349e8a1 to 7c238cf Compare April 8, 2024 14:01
@potiuk

potiuk commented Apr 8, 2024

Copy link
Copy Markdown
Member

Rebased it -> your PR was 62 commits behind -> next time you can do it yourself too (see our contributing docs and search for rebase) https://github.com/apache/airflow/blob/main/contributing-docs/10_working_with_git.rst

@Lee-W

Lee-W commented Apr 8, 2024

Copy link
Copy Markdown
Member

Thanks @potiuk for help out!

@SubhamSinghal There's one static check failure. We might need your help to fix it

Lee-W
Lee-W approved these changes Apr 8, 2024
View reviewed changes

@Lee-W Lee-W left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM.

Comment thread airflow/providers/databricks/hooks/databricks.py Outdated
@Lee-W Lee-W requested a review from pankajkoti April 8, 2024 14:36
pankajkoti
pankajkoti approved these changes Apr 9, 2024
View reviewed changes

@pankajkoti pankajkoti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One minor suggestion inline to optimise the number of conditional checks.

Comment thread airflow/providers/databricks/operators/databricks.py Outdated
@pankajkoti pankajkoti merged commit 6f21f7d into apache:main Apr 10, 2024
@eladkal eladkal mentioned this pull request Apr 10, 2024
Closed
@SubhamSinghal SubhamSinghal deleted the bugfix/create-job-does-not-update-acl branch April 11, 2024 08:07
@SubhamSinghal SubhamSinghal restored the bugfix/create-job-does-not-update-acl branch April 11, 2024 08:10
This was referenced Apr 13, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Taragolis Taragolis Taragolis left review comments
@Lee-W Lee-W Lee-W approved these changes
@pankajkoti pankajkoti pankajkoti approved these changes
+1 more reviewer
@subham611 subham611 subham611 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:providers provider:databricks

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CreateJobOperator does not udpdate ACL

6 participants

@SubhamSinghal @subham611 @Lee-W @potiuk @Taragolis @pankajkoti